Remove provides+required GVK and modify/remove associated tests #408
Conversation
openshift-ci
bot
added
the
do-not-merge/work-in-progress
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #408 +/- ##
==========================================
+ Coverage 83.68% 83.97% +0.28%
==========================================
Files 27 27
Lines 1103 1073 -30
==========================================
- Hits 923 901 -22
+ Misses 123 119 -4
+ Partials 57 53 -4
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
Signed-off-by: dtfranz <[email protected]>
dtfranz
force-pushed
the
remove_required_provided_gvks
branch
from
5f33a41 to
ea08115
Compare
Signed-off-by: dtfranz <[email protected]>
openshift-ci
bot
removed
the
do-not-merge/work-in-progress
| const ( | ||
| MediaTypePlain = "plain+v0" | ||
| MediaTypeRegistry = "registry+v1" | ||
| PropertyBundleMediaType = "olm.bundle.mediatype" | ||
| ) |
There was a problem hiding this comment.
I was a bit confused since this (and PackageRequired an some other bits in this file) is not related to GVK removal. But we still need these changes for further entity source removal.
I'm ok to merge it like this.
| assert.Nil(t, gvk) | ||
| }) | ||
| }}, | ||
| wantRequiredPackages: nil, |
There was a problem hiding this comment.
Nit: empty value for a pointer is nil so this can be ommited.
| b.mu.Lock() | ||
| defer b.mu.Unlock() | ||
| if b.requiredGVKs == nil { | ||
| requiredGVKs, err := loadFromProps[[]GVKRequired](b, property.TypeGVKRequired, false) | ||
| if b.mediaType == "" { |
There was a problem hiding this comment.
Hm. We should probably use a pointer here: if nil - try to unmarshal. If non-nil - then we already tried to load it (even if the string itself is empty).
There was a problem hiding this comment.
Created #409 for this. Will rebase once this PR gets merged.
GVK Properties are not being set on bundle properties, and how we handle these dependencies has not been fully fleshed out. Removing this code will also simplify the transition away from Entity/EntitySource code.
Please also see slack discussion here.